Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get logs #1028

Merged
merged 24 commits into from
Sep 3, 2020
Merged

Get logs #1028

merged 24 commits into from
Sep 3, 2020

Conversation

JekaMas
Copy link
Contributor

@JekaMas JekaMas commented Sep 1, 2020

closes #937

@akme It'd be great if you check the getLogs API.

@AlexeyAkhunov
Copy link
Contributor

Could you merge the master in and get the CI green? I will merge it because it contains some code improvements, and we can address any further issues separarely

// is compatible with the proto package it is being compiled against.
// A compilation error at this line likely means your copy of the
// proto package needs to be updated.
const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a downgrade?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on what generator version installed locally.

But i found solution for this: if put exact version to go.mod and install generator by ‘go install’ from app folder (make devtools - does it now) - then it installs version from go.mod file.

So, if run make devtools in current master and re-gen things, then ProtoPackageIsVersion6 must appear

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I've cleaned module packages and downloaded them once again, and now it generates "SupportPackageIsVersion7". At the end of the day, I updated grpc package and regenerated all files once again. It seems everything is good now and tests are green

@akme
Copy link

akme commented Sep 2, 2020

Request:

curl --data '{"jsonrpc":"2.0","method":"eth_getLogs","params":[{"address":"0x9feb49ac78ec80838665d7ab75e43cfbdc4f4277","fromBlock":"0x0","toBlock":"0x0","topics":[["0x4828028875d55b3eee8f0792f7db619443d485cdf943daabb66aa69b59943d60","0xedfff599186e5df9f7c27e1a4c7ccace9b82f62bf814d7e98b10e07b80d677ce","0x8a0df8ef054fae2c3d2d19a7b322e864870cc9fd3cb07fb9526309c596244bf4","0xb2b3cbe033aa38e93c0eb1ceee79d790e60008cbc021b10eed86e2fa214ae645","0x003195cc3cce6bd8cab724dba36b9fcdbd8b093db6771402b7b0ff3637f79070"]]}],"id":623007}' -H "Content-Type: application/json" -X POST

Response with error:

{"jsonrpc":"2.0","id":623007,"error":{"code":-32602,"message":"invalid argument 0: math/big: cannot unmarshal \"\\\"0x0\\\"\" into a *big.Int"}}

@AlexeyAkhunov
Copy link
Contributor

thanks for checking, @akme ! I moved your comment into the issue, because I want to merge this and we can continue fixing in a new PR

if len(logs) > 0 {
// We have matching logs, check if we need to resolve full logs via the light client
if logs[0].TxHash == (common.Hash{}) {
receipts := rawdb.ReadReceipts(api.dbReader, header.Hash(), header.Number.Uint64(), params.MainnetChainConfig)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the hard-coded config? There is a way to read it from the DB (you can see in the getReceipt method or similar)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -3,6 +3,7 @@ package commands
import (
"context"
"fmt"
ethereum "github.com/ledgerwatch/turbo-geth"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I've never seen that one before, what does it import?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found it. It is interfaces.go

@AlexeyAkhunov
Copy link
Contributor

@JekaMas I think you forgot to include the function UnmarshalJSON for your FilterCriteria, that is why we cannot parse block numbers. See how it is done in eth/filters/api.go:483. There is an alias FilterCriteria declared, which is used in the signature of the RPC function, and that alias needs to have UnmarshalJSON function to custom parsing

@JekaMas
Copy link
Contributor Author

JekaMas commented Sep 2, 2020

@akme nice catch, could you try again in the latest commit?

end = crit.ToBlock.Int64()
}

if begin > end {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure we need to error out when being == end or just return empty result?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would check pre-existing behaviour

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I see go-ethereum just ignores this case.

@@ -502,11 +501,11 @@ func (args *FilterCriteria) UnmarshalJSON(data []byte) error {
args.BlockHash = raw.BlockHash
} else {
if raw.FromBlock != nil {
args.FromBlock = big.NewInt(raw.FromBlock.Int64())
args.FromBlock = rpc.NewRPCBlockNumber(int(raw.FromBlock.Int64()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't raw.FromBlock and args.FromBlock of the same type, can you just do assignment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, golang doesn't allow & operation for type conversion but we have to convert int64 to int or rpc.BlockNumber.

interfaces.go Outdated
@@ -134,8 +135,8 @@ type ContractCaller interface {
// FilterQuery contains options for contract log filtering.
type FilterQuery struct {
BlockHash *common.Hash // used by eth_getLogs, return logs only from block with this hash
FromBlock *big.Int // beginning of the queried range, nil means genesis block
ToBlock *big.Int // end of the range, nil means latest block
FromBlock *rpc.BlockNumber // beginning of the queried range, nil means genesis block
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are defining FromBlock and ToBlock to be of these types, perhaps you don't need UnmarshalJSON function for FilterQuery type anymore? My understanding is that it only existed to convert *rpc.BlockNumber to *big.Int

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still need UnmarshalJSON for Account that can be either a single value or a slice

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted this type change

rpc/types.go Outdated
@@ -69,6 +69,11 @@ type jsonWriter interface {

type BlockNumber int64

func NewRPCBlockNumber(n int) *BlockNumber {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you still need this function?

go.sum Outdated
@@ -432,8 +432,8 @@ google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ij
google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
google.golang.org/grpc v1.30.1 h1:oJTcovwKSu7V3TaBKd0/AXOuJVHjTdGTutbMHIOgVEQ=
google.golang.org/grpc v1.30.1/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak=
google.golang.org/grpc v1.33.0-dev h1:c0EY3sGPLj50wEdGQDpiS3zvk/zdduzrAkJTfa9ocjY=
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is dev an unstable version?

Copy link
Contributor Author

@JekaMas JekaMas Sep 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AskAlexSharov I think only you have the right package and libs versions, I've tried to install versions from master and run devtools many times and always get packageVersion7.
Could you checkout to this branch and regenerate files from proto files and then push results to the branch?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So.

  1. "Is dev an unstable version?" - 1.31 their current release, 1.32-dev their next release, 1.33-dev their next-next release.
  2. Go code generates by google.golang.org/grpc/cmd/protoc-gen-go-grpc - it means we looking for version reproducibility of this particular tool.
  3. When I run make devtools in master - it added line google.golang.org/grpc/cmd/protoc-gen-go-grpc to go.mod file (I thought that I added this live with exact version already). Reason why it's not there: go mod tidy removes it because our source code doesn't use it - it's true - we don't use this package from source code, but we use it to generate go code.
  4. So, probably we can trick go mod tidy - add dummy package where I will use all our binary dependencies. But nope, we can do it only if such dep has non-main package: cmd/go: accept main packages as dependencies in go.mod files golang/go#32504
  5. But I found how to trick it anyway: Reproducible versions of binary dependencies #1037
  6. I pointed google.golang.org/grpc/cmd/protoc-gen-go-grpc to commit of v1.30.1 tag

@akme
Copy link

akme commented Sep 2, 2020

@akme nice catch, could you try again in the latest commit?

worked well

@AskAlexSharov
Copy link
Collaborator

Hi, guys.
I merged #1037 to master
then merged master to this branch
then regenerated things with right versions
but need help to test - because if DB has no receipts it returns empty slice

@AlexeyAkhunov AlexeyAkhunov merged commit e4f495f into master Sep 3, 2020
@AlexeyAkhunov AlexeyAkhunov deleted the getLogs branch September 29, 2020 15:02
battlmonstr pushed a commit that referenced this pull request Sep 14, 2023
cffls pushed a commit to cffls/erigon that referenced this pull request Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eth_getLogs doesn't work
4 participants